-
-
Notifications
You must be signed in to change notification settings - Fork 42
London | 25-SDC-Nov | Jesus del Moral | Sprint 5 | Implement Laptop Allocation #297
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
London | 25-SDC-Nov | Jesus del Moral | Sprint 5 | Implement Laptop Allocation #297
Conversation
0a7596e to
3a77f8f
Compare
OracPrime
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of minor points, one bug, but generally good code, well done.
laptop-allocation.py
Outdated
| class Person: | ||
| name: str | ||
| age: int | ||
| preferred_operating_system: Tuple[OperatingSystem, ...] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Why a Tuple, not a List?
- If it is a collection, it should have a pluralised name (preferred_operating_systems)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1.- I used Tuple instead of List to signal immutability and a fixed relationship, not something that will be modified. Items should not change.
2.- I have changed the name "preferred_operating_systems"
|
|
||
| def allocate_laptops(people: List[Person], laptops: List[Laptop]) -> Dict[Person, Laptop]: | ||
| if len(people) != len(laptops): | ||
| raise ValueError("Number of people must equal number of laptops.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does the number of people need to equal the number of laptops?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this implementation, the number of people must equal the number of laptops because the algorithm assumes a one to one assignment, every person receives exactly one laptop and every laptop is assigned to exactly one person.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree. Everyone needs a laptop, but spare laptops aren't a problem.
| raise ValueError("Number of people must equal number of laptops.") | ||
|
|
||
| # Clone list so we can remove laptops as they are assigned | ||
| available_laptops = laptops.copy() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good spot!
Laptop allocation app
Every person should be allocated exactly one laptop.
If we define “sadness” as the number of places down in someone’s ranking the operating system the ended up with (i.e. if your preferences were [UBUNTU, ARCH, MACOS] and you were allocated a MACOS machine your sadness would be 2), we want to minimise the total sadness of all people. If we allocate someone a laptop with an operating system not in their preferred list, treat them as having a sadness of 100